[WRONG BRANCH] fix(claude): isolate passthrough credentials - #73
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
⏳ DRAFT
What to do
Its title has been prefixed with |
|
✅ Deterministic PR hygiene checks passed. |
📝 WalkthroughWalkthroughNative Claude passthrough now separates proxy admission authentication from Anthropic provider credentials. Exposed listeners require ChangesNative Claude passthrough authentication
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant NativePassthrough
participant AdmissionValidation
participant AnthropicUpstream
Client->>NativePassthrough: Submit native Claude request
NativePassthrough->>AdmissionValidation: Validate x-opencodex-api-key when listener is exposed
AdmissionValidation-->>NativePassthrough: Return admission result
NativePassthrough->>NativePassthrough: Remove proxy admission credentials
NativePassthrough->>AnthropicUpstream: Forward provider credentials and allowed headers
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/claude-native-passthrough.test.ts`:
- Around line 230-245: Add a second successful dedicated passthrough request in
this test, using the admission secret in authorization, a provider credential in
x-api-key, and the valid x-opencodex-api-key. Assert the request succeeds and
the captured upstream request removes authorization while retaining the provider
x-api-key; keep the existing x-opencodex-api-key filtering assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: afaab7aa-54f5-4aa3-a7a6-61545bc20d4f
📒 Files selected for processing (5)
docs-site/src/content/docs/guides/claude-code.mddocs-site/src/content/docs/ko/guides/claude-code.mddocs-site/src/content/docs/zh-cn/guides/claude-code.mdsrc/server/claude-messages.tstests/claude-native-passthrough.test.ts
| const dedicated = await globalThis.fetch(url, { | ||
| method: "POST", | ||
| headers: { | ||
| "content-type": "application/json", | ||
| "x-opencodex-api-key": "sk-ant-api03-key", | ||
| "authorization": "Bearer sk-ant-oat01-tst", | ||
| "x-api-key": "sk-ant-api03-key", | ||
| }, | ||
| body: JSON.stringify(claudeBody()), | ||
| }); | ||
| expect(dedicated.status).toBe(200); | ||
| await dedicated.text(); | ||
| expect(captured).toHaveLength(1); | ||
| expect(captured[0].headers.get("authorization")).toBe("Bearer sk-ant-oat01-tst"); | ||
| expect(captured[0].headers.get("x-api-key")).toBeNull(); | ||
| expect(captured[0].headers.get("x-opencodex-api-key")).toBeNull(); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Add a successful passthrough case for authorization filtering.
Lines 243-245 only verify removal of an admission secret from x-api-key. The rejected request does not enter anthropicNativePassthrough, so it does not test the authorization branch at src/server/claude-messages.ts lines 337-340.
Send a second dedicated request with the admission secret in authorization, a provider credential in x-api-key, and the valid x-opencodex-api-key. Assert that the upstream receives no authorization header and retains the provider x-api-key.
Proposed regression case
+ const authorizationAdmission = await globalThis.fetch(url, {
+ method: "POST",
+ headers: {
+ "content-type": "application/json",
+ "x-opencodex-api-key": "sk-ant-api03-key",
+ "authorization": "Bearer sk-ant-api03-key",
+ "x-api-key": "sk-ant-oat01-tst",
+ },
+ body: JSON.stringify(claudeBody()),
+ });
+ expect(authorizationAdmission.status).toBe(200);
+ await authorizationAdmission.text();
+ expect(captured).toHaveLength(2);
+ expect(captured[1].headers.get("authorization")).toBeNull();
+ expect(captured[1].headers.get("x-api-key")).toBe("sk-ant-oat01-tst");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const dedicated = await globalThis.fetch(url, { | |
| method: "POST", | |
| headers: { | |
| "content-type": "application/json", | |
| "x-opencodex-api-key": "sk-ant-api03-key", | |
| "authorization": "Bearer sk-ant-oat01-tst", | |
| "x-api-key": "sk-ant-api03-key", | |
| }, | |
| body: JSON.stringify(claudeBody()), | |
| }); | |
| expect(dedicated.status).toBe(200); | |
| await dedicated.text(); | |
| expect(captured).toHaveLength(1); | |
| expect(captured[0].headers.get("authorization")).toBe("Bearer sk-ant-oat01-tst"); | |
| expect(captured[0].headers.get("x-api-key")).toBeNull(); | |
| expect(captured[0].headers.get("x-opencodex-api-key")).toBeNull(); | |
| const dedicated = await globalThis.fetch(url, { | |
| method: "POST", | |
| headers: { | |
| "content-type": "application/json", | |
| "x-opencodex-api-key": "sk-ant-api03-key", | |
| "authorization": "Bearer sk-ant-oat01-tst", | |
| "x-api-key": "sk-ant-api03-key", | |
| }, | |
| body: JSON.stringify(claudeBody()), | |
| }); | |
| expect(dedicated.status).toBe(200); | |
| await dedicated.text(); | |
| expect(captured).toHaveLength(1); | |
| expect(captured[0].headers.get("authorization")).toBe("Bearer sk-ant-oat01-tst"); | |
| expect(captured[0].headers.get("x-api-key")).toBeNull(); | |
| expect(captured[0].headers.get("x-opencodex-api-key")).toBeNull(); | |
| const authorizationAdmission = await globalThis.fetch(url, { | |
| method: "POST", | |
| headers: { | |
| "content-type": "application/json", | |
| "x-opencodex-api-key": "sk-ant-api03-key", | |
| "authorization": "Bearer sk-ant-api03-key", | |
| "x-api-key": "sk-ant-oat01-tst", | |
| }, | |
| body: JSON.stringify(claudeBody()), | |
| }); | |
| expect(authorizationAdmission.status).toBe(200); | |
| await authorizationAdmission.text(); | |
| expect(captured).toHaveLength(2); | |
| expect(captured[1].headers.get("authorization")).toBeNull(); | |
| expect(captured[1].headers.get("x-api-key")).toBe("sk-ant-oat01-tst"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/claude-native-passthrough.test.ts` around lines 230 - 245, Add a second
successful dedicated passthrough request in this test, using the admission
secret in authorization, a provider credential in x-api-key, and the valid
x-opencodex-api-key. Assert the request succeeds and the captured upstream
request removes authorization while retaining the provider x-api-key; keep the
existing x-opencodex-api-key filtering assertions.
Source: Path instructions
|
Superseded by the current-dev upstream implementation in lidge-jun#1536. The replacement preserves the credential-isolation intent, adds listener-effective policy handling, fails closed on duplicate credential headers, and includes the inverse provider x-api-key regression requested by CodeRabbit. Closing this stale wrong-branch draft without deleting its branch. |
Motivation
AuthorizationorX-Api-Key, which could leak management/data-plane credentials to upstream providers./api/*management routes.Description
src/server/claude-messages.tsby importing auth helpers and changing credential detection soAuthorization/X-Api-Keyvalues that match OpenCodex admission secrets are ignored as provider credentials and are not forwarded upstream.X-OpenCodex-API-Keyon non-loopback listeners for native passthrough requests, so non-loopback passthrough only activates when admission is presented via the dedicated header.AuthorizationorX-Api-Keybefore copying headers to the upstream Anthropic request to ensure proxy admission secrets are never forwarded.tests/claude-native-passthrough.test.tsthat verifies ambiguous legacy-header usage is rejected and the dedicated-header path still succeeds, and updated the native passthrough docs (English/Korean/Simplified Chinese) to document header handling and the non-loopback requirement.Testing
bun run typecheckand it passed.bun run test -- tests/claude-native-passthrough.test.tsand they passed (11 tests, 0 failures).bun run privacy:scanand the scan passed after replacing test fixture tokens with non-sensitive placeholders.bun run testrun that exercised many suites locally and observed passing results for the exercised tests, but a full docs-site build (cd docs-site && bun install --frozen-lockfile && bun run build) was blocked by external npm registry 403 errors so the site build could not be completed in this environment.Summary by CodeRabbit
Bug Fixes
x-opencodex-api-keyheader.Documentation
Tests